Make input loading fallible in SyncFromDiskStage #3195
Conversation
…e if it is encountered
libafl_bolts/src/lib.rs
Outdated
| @@ -341,6 +341,8 @@ pub enum Error { | |||
| InvalidCorpus(String, ErrorBacktrace), | |||
| /// Error specific to a runtime like QEMU or Frida | |||
| Runtime(String, ErrorBacktrace), | |||
| /// The `Input` was invalid. | |||
| InvalidInput(ErrorBacktrace), | |||
There was a problem hiding this comment.
Wouldn't it make sense to include a string here? Like, the path of the invalid input, and maybe a root cause if there is any?
There was a problem hiding this comment.
Especially given the callback is user provided, adding a string will make things way easier to debug IMHO. The callback can differentiate between different invalid input causes
libafl/src/stages/sync.rs
Outdated
| let input = match (self.load_callback)(fuzzer, state, &path) { | ||
| Ok(input) => input, | ||
| Err(Error::InvalidInput(reason, _)) => { | ||
| log::debug!( |
There was a problem hiding this comment.
Or maybe info if you think it's expected
libafl_bolts/src/lib.rs
Outdated
| @@ -1064,7 +1081,7 @@ pub fn get_thread_id() -> u64 { | |||
| #[allow(clippy::cast_sign_loss)] | |||
| /// Return thread ID without using TLS | |||
| pub fn get_thread_id() -> u64 { | |||
| use libc::{SYS_gettid, syscall}; | |||
| use libc::{syscall, SYS_gettid}; | |||
There was a problem hiding this comment.
is this a cargo fmt update of some sorts?
There was a problem hiding this comment.
Seems like your fmt is not the same as the CI one
|
Change debug to info and figure out how to make CI happy, then this is good to go <3 |
|
looks like this only needs a cargo fmt with an updated rust |
|
Thank you! CI is no longer your fault at this point :) |
| @@ -58,6 +58,7 @@ impl SyncFromDiskMetadata { | |||
| } | |||
|
|
|||
| /// A stage that loads testcases from disk to sync with other fuzzers such as AFL++ | |||
| /// When syncing, the stage will ignore `Error::InvalidInput` and will skip the file. | |||
There was a problem hiding this comment.
Can you link to the error like so
[`Error::InvalidInput`]
Description
This PR introduces the
InvalidInputvariant inError. When syncing inSyncFromDiskStage, if such an error is encountered, it is logged and the file is skipped.Reasoning:
In the case of a grammar fuzzer loading inputs from other fuzzers, the inputs may be structurally invalid. This should not make it an error in the syncing process
Checklist
./scripts/precommit.shand addressed all comments